Added type inferencing for Vector & Matrix operations in multiply.js (+performance boost!) #3149

RandomGamingDev · 2024-02-07T02:47:12Z

The Issue

mathjs has had an "issue" with performance (some of the first search results for "mathjs is slow":

Except it doesn't.

Why the slow performance and all the complaints then?

mathjs is quite powerful, but the idea that you should explicitly set the numeric type of your tensors is something that people simply don't come across when reading the documentation (here are some pages from the documentation):

In fact, mathjs's documentation makes it look like explicitly defining the numeric type is completely unnecessary because of the default type assignment in the config of mathjs:

https://mathjs.org/docs/datatypes/numbers.html
"Most functions can determine the type of output from the type of input: a number as input will return a number as output, a BigNumber as input returns a BigNumber as output. Functions which cannot determine the type of output from the input (for example math.evaluate) use the default number type, which can be configured when instantiating math.js"

This leads to extremely poor performance when people try to use mathjs:

Rendering 1000 hyper-cubes rotated around 2 axis using vanilla mathjs, no explicit numeric types (slow & avgs ~40ms/frame on my machine): https://editor.p5js.org/PotatoBoy/sketches/uPZZwvhgC

Which wouldn't exist had these explicit numeric types been used:

Rendering 1000 hyper-cubes rotated around 2 axis using vanilla mathjs with explicit numeric types (fast & avgs ~1-2ms/frame on my machine): https://editor.p5js.org/PotatoBoy/sketches/DsDHNojgz

Why this wasn't done before now and how this PR fixes the issue that caused it

It seems like adding type inferencing was previously considered, but then ignored because of an issue with changing element types:

This PR fixes this by getting the type during the tensor operation itself

<tensor>.getDataType() is called to infer the datatype if it wasn't explicitly defined
While getting inferring the type every operation may seem slow, compared to the alternative of using the generic function, it's much much faster as seen by the previous examples and the solution's example and is well worth it for the performance.
The datatype of the generated tensor using the inferred datatype is changed right before returning it, but not for the instantiation parameter in order to future-proof it in case future optimized methods of loading tensor data are used so that they can be used for the operation.

The Solution this PR provides

Type inferencing boosts the performance to speeds nearly indistinguishable from the speeds of explicit types

Rendering 1000 hyper-cubes rotated around 2 axis using type inferencing mathjs, no explicit numeric types (fast & avgs ~1-2ms/frame on my machine): https://editor.p5js.org/PotatoBoy/sketches/01IsuOlTR

Lastly, I'd still recommend that the documentation more readily states and promotes the existence of these explicit types, but I'm not sure whether or not that would fit in with what mathjs is trying to go for in its documentation, and it mostly likely won't be necessary with type inferencing added so I won't be adding documentation doing that in this PR.

…the explicitly defined type is returned at the end

josdejong · 2024-02-07T08:56:34Z

Thanks for working out this PR @RandomGamingDev . You're right, matrix operations are way faster when there is a datatype configured in the matrix. Currently the default is to leave it undefined and assume the matrix may have mixed content, which is slow.

So, good point to see how we can address this and make the "default" performance better.

Some thoughts:

Your solution to improve multiply will work, but only for multiply, not for other matrix operations.
So, I think we should look for a more generic solution.
The most logic one is to ensure call getArrayDataType(data) when constructing the matrix, ensuring it always has _datatype defined.
This will make creating a matrix slower, and I would love to have an idea on the performance impact: how much slower matrix creation becomes compared to how much we win with typical matrix operations (multiplication, addition, etc).

@gwhitney do you have any thoughts in this regard?

RandomGamingDev · 2024-02-07T14:30:56Z

The most logic one is to ensure call getArrayDataType(data) when constructing the matrix, ensuring it always has _datatype defined.
This will make creating a matrix slower, and I would love to have an idea on the performance impact: how much slower matrix creation becomes compared to how much we win with typical matrix operations (multiplication, addition, etc).

The issue with that is the reason why this wasn't done before when talked about in the previous issue which is mathjs users might change the numeric type of part of the tensor and then be confused why it isn't working:

It seems like adding type inferencing was previously considered, but then ignored because of an issue with changing element types:

Also, the performance issue of getting the numeric type every operation is offset by the performance gained.

Your solution to improve multiply will work, but only for multiply, not for other matrix operations.
So, I think we should look for a more generic solution.

Yes, it's true that this PR will only improve multiply, but I don't think a more generic solution would work well. This solution is already generic enough and basically the only other operations that I see that benefit from type inferencing are here:

The type inferencing solution works for these as well. I'll add type inferencing for them as well.

gwhitney · 2024-02-07T17:34:20Z

I could believe either of the following could be a reasonable path forward

(a) datatype be set on creation, and then any operation that would make it mixed fails unless the type is explicitly set to mixed, because we all know that 99.9% of the use is not actually mixed type; or

(b) @RandomGamingDev is right and it only matters for a handful of operations and so it's OK just to check for uniform type on every call to such operations.

Conceptually, I agree with Jos that (a) is "cleaner" but it may not be worth the trouble. One question: if we go (b), is there a minimum size for the matrix below which it's not worth checking (because we don't get back enough performance payoff to make it worth the check? Just wondering. Maybe multiplying a mixed type matrix is just so bad that even for 2x2 matrices, it's worth checking for uniform type.

RandomGamingDev · 2024-02-07T18:41:30Z

Conceptually, I agree with Jos that (a) is "cleaner" but it may not be worth the trouble.

The main issue I have with (a) is with backwards compatibility and debugging and because the _data parameter (despite looking like it should be private due to the prepended underscore) is modified and used directly in programs using mathjs. Changing it so that changing the types causes an error would break back-compatibility for these programs and would mean having to detect variable modifications as soon as they happen (since it sounds like you want the wrong modification of a tensor to instantly throw an error), which was the issue that stopped type inferencing from getting added in the first place and although it appears possible using certain features of JS, it looks like an overly bulky and unnecessary solution to me. Because of these reasons I don't think that (a)'s choice of AOT (ahead of time) type inferencing on tensor creation would work as well as (b)'s choice of JIT (just in time) type inferencing on tensor operation.

One question: if we go (b), is there a minimum size for the matrix below which it's not worth checking (because we don't get back enough performance payoff to make it worth the check? Just wondering. Maybe multiplying a mixed type matrix is just so bad that even for 2x2 matrices, it's worth checking for uniform type.

It might not be that bad, but even for a 2x2 matrix multiplication that's still using the general function and inferring the type for every single operation, so realistically the JIT method of just checking at the start using the decently efficient <tensor>.getDataType() method is going to be the more efficient option unless the type is mixed, in which case the performance difference is tiny compared to the cost of using the general function to infer the type for every number of the tensor which is a change that I'd say is well worth it if it means that all other tensors without an explicit numeric type get multitudes faster and so that newcomers to the library won't get confused by the "terrible" performance.

gwhitney · 2024-02-07T18:46:19Z

the _data parameter (despite looking like it should be private due to the prepended underscore) is modified and used directly in programs using mathjs.

That's surely not a "supported" use of the mathjs api, or at least it very much should not be. The existence of such usage shouldn't be a factor in the choice between (a) and (b). That's not saying I am pushing hard for (a) over (b); I am leaving that up to Jos, and I'd support either direction. I don't see that it's too costly for the official api to check any matrix-modifying calls to see if they respect its current datatype.

RandomGamingDev · 2024-02-07T22:34:58Z

That's surely not a "supported" use of the mathjs api, or at least it very much should not be.

Yeah I agree that it shouldn't be since it should be private as shown by the prepended underscore.

The existence of such usage shouldn't be a factor in the choice between (a) and (b).

Might be a bit of a controversial opinion for some people, but I respect not respecting bad and illegal usage of a library lol

However, despite the proper setter being documented, it can be hard for many to find since it doesn't have its own section, nor an emphasis on its importance, and I think that it's important to be aware of it since the fact is that such usage is not rare, but common (here are a few examples from just a quick google search):

I am leaving that up to Jos, and I'd support either direction.

I also support both as long as something happens to make operations with untyped tensors faster since it's causing a lot frustration with math.js, especially from newcomers, although I do still believe that the JIT approach from this PR is the best route to go and comes with little if any issues.

I'll add type inferencing to the other operations in this PR if we get the go-ahead from @josdejong.

gwhitney · 2024-02-07T22:46:53Z

it can be hard for many to find since it doesn't have its own section,

Well, I mean these days there is https://mathjs.org/docs/datatypes/matrices.html#getting-and-setting-a-value-in-a-matrix
And many of your links explicitly say "well, you could look at _data, but you really shouldn't" -- and they are right! So I maintain my position that these direct-usages-of-internals should not be a factor in Jos's decision, but that nevertheless, this could go either way.

RandomGamingDev · 2024-02-07T23:34:17Z

Well, I mean these days there is https://mathjs.org/docs/datatypes/matrices.html#getting-and-setting-a-value-in-a-matrix

I meant giving it its own page or some extra emphasis in the matrices section so that they see it before they decide to just print out the tensor and use _data

And many of your links explicitly say "well, you could look at _data, but you really shouldn't" -- and they are right!

Yes, and again:

I respect not respecting bad and illegal usage of a library lol

I just wanted to give more information to be aware of regarding how common this usage is whether or not it plays a role in the end decision.

josdejong · 2024-02-09T13:36:15Z

Thanks for your inputs guys. I'll give this some more thought next week.

I would like to get a more clear picture on the tradeoffs and how this will work out for typical use cases. If we save more in matrix operations than it costs to run getMatixType it can be worth it, but at this point it is a bit of a wild guess to me. A bit of benchmarking to get a feel for this would be helpful. And alongside that, I want to get a clear picture on whether this is only interesting for multiply or for many operations (I expect the latter).

RandomGamingDev · 2024-02-09T13:59:09Z

And alongside that, I want to get a clear picture on whether this is only interesting for multiply or for many operations (I expect the latter).

#3149 (comment)
The other operations that I see that benefit from type inferencing are here (if you choose to go down the JIT path I'll implement type inferencing for them right away :D):

josdejong

@RandomGamingDev I've been doing some performance testing and had a better look at your PR. This looks really promising, I can't wait to get this PR finished and merged 😎 .

Some thoughts/questions:

I didn't find any performance issues with most functions or operators. I noticed though that the .map() method of DenseMatrix is relatively slow. This is a off topic I think, we should address that in a separate PR (if there is room for improvement). In any case, I would like to proceed with this PR.
The functions multiply, divide, and pow are quite slow. Your PR improves the performance of all three of these functions dramatically, this awesome!
Do you see low hanging fruit to improve divide too? 😁
I made some inline comments in the code, can you have a look at those?

src/function/arithmetic/multiply.js

… and src/function/matrix/dot.js

RandomGamingDev · 2024-02-18T04:06:14Z

@RandomGamingDev I've been doing some performance testing and had a better look at your PR. This looks really promising, I can't wait to get this PR finished and merged 😎 .

Some thoughts/questions:
1. I didn't find any performance issues with most functions or operators. I noticed though that the .map() method of DenseMatrix is relatively slow. This is a off topic I think, we should address that in a separate PR (if there is room for improvement). In any case, I would like to proceed with this PR.
2. The functions multiply, divide, and pow are quite slow. Your PR improves the performance of all three of these functions dramatically, this awesome!
3. Do you see low hanging fruit to improve divide too? 😁
4. I made some inline comments in the code, can you have a look at those?

Yeah, that sounds like something that could use improvement, especially if a lot of other functions use it for iteration.
:D
As you said previously, the performance has already been improved. As for improving it further, I don't see any specific way to improve the performance of matrix division with just the type inferencing in this PR.
I've taken a look at them and added them

I'd like to note that I only added type inferencing to some of the matrix algorithms in the matrix utils since some of the callbacks used alongside the algorithms that didn't have type inferencing added to them aren't typed and thus fail when type inferencing tries to assume a type. Maybe we should change those in the future, but for now I've left those without type inferencing.

josdejong

Thanks for the updates and for handling more similar cases!

Let's indeed leave this PR as is. We can indeed always see if there is more room for improvement later.

There are still three cases left of if statements changing the _datatype property later on, can you also move those to the place where the matrix is created? (see inline comments).

src/function/arithmetic/multiply.js

…ix creation in multiply

josdejong

Thanks! Merging your PR now.

josdejong · 2024-02-22T15:59:22Z

Published now in v12.4.0, thanks again!

* fix: Find eigenvectors of defective matrices (josdejong#3037) * fix: Find eigenvectors of defective matrices Previously, attempting to take the `eigs` of any defective matrix was doomed to fail in an attempt to solve a singular linear system. This PR detects the situation (as best as it can given the inherent numerical instability of the current methods used) and handles it. Note that in such cases, it's not possible to return a square matrix whose columns are the eigenvectors corresponding to the returned eigenvalues. In light of that fact and issue josdejong#3014, this PR also changes the return value of `eigs` so that the eigenvectors are passed back in a property `eigenvectors` which is an array of plain objects `{value: e, vector: v}`. Note that this PR makes the ancillary changes of correcting the spelling of the filename which was "realSymetric.js," and replacing the now-unnecessary auxiliary function "createArray" therein with `Array(size).fill(element)`. The rationale for performing these changes not strictly related to the issues at hand is that this file is rarely touched and with the level of maintenance hours we have at hand, it's more efficient to do these small refactorings in parallel with the actual bugfixes, which are orthogonal and so will not be obfuscated by this refactor. Note `git diff` does properly track the file name change. However, it also makes a potentially more pervasive change: in order for the numerically-sensitive algorithm to work, it changes the condition on when two very close (double) numbers are "nearlyEqual" from differing by less than DBL_EPSILON to differing by less than or equal to DBL_EPSILON. Although this may change other behaviors than the ones primarily being addressed, I believe it is an acceptable change because (a) It preserves all tests. (b) DBL_EPSILON is well below the standard config.epsilon anyway (c) I believe there are extant issues noting the odd/inconsistent behavior of nearlyEqual near 0 anyway, so I believe this will be overhauled in the future in any case. If so, the eigenvector computation will make a good test that a future nearlyEqual algorithm is working well. To be clear, the direct motivation for the change is that there are multiple cases in the eigenvector computation in which a coefficient that is "supposed" to be zero comes out to precisely DBL_EPSILON, which is fairly unsurprising given that these coefficients are produced by subtracting an eigenvalue from a diagonal entry of a matrix, which is likely to be essentially equal to that eigenvalue. As many tests of defective matrices as I could readily find by web searching have been added as unit tests (and one more in the typescript type testing). An additional case I found still fails, but in the _eigenvalue_ computation rather than the _eigenvector_ search, so that was deemed beyond the scope of this PR and has been filed as issue josdejong#3036. Resolves josdejong#2879. Resolves josdejong#2927. Resolves josdejong#3014. * refactor: remove comma that lint now doesn't like * test: add a test for eigs with a precision argument * feat: Use simple shifts in QR eigenvalue iterations that improve convergence Although we might want to use better shifts in the future, we might just use a library instead. But for now I think this: Resolves josdejong#2178. Also responds to the review feedback provided in PR josdejong#3037. * docs: update history * fix: josdejong#3074 improve error message when using function `max` in `derivative` * fix: josdejong#3073 parsing quotes inside a string * fix: josdejong#2027 cannot use named operators like `to` or `mod` as property name * chore: update devDependencies * chore: run `npm audit fix` * chore: publish v11.11.2 * Drop official support for Node.js 14 and 16 * fix: change toTex variable and function assignment from `:=` to `=` (see josdejong#2980, josdejong#3032) * Update history * docs: fix typo in `p.set` example in the documentation of matrices (josdejong#3080) * feat: Add option to eigs() to turn off eigenvector computation (josdejong#3057) * feat: Add option to eigs() to turn off eigenvector computation For large matrices, the eigenvector computation can be noticeably expensive and so it's worthwhile to have a way to turn it off if the eigenvectors will not be used. Resolves josdejong#2180. * fix: Add test for precision in options arg of eigs And also a fix for a small bug that the new test uncovered. * test: check eigs with matrix and options * refactor: remove dead code from complexEigs.js * fix: add new signatures of eigs to typescript * test: ensure eigenvectors property not present with eigenvectors: false option * fix: correct balancing code in complexEigs * Fix: josdejong#3073 escaping in strings (josdejong#3082) * chore: refactor parsing strings to not rely on `JSON.parse` * fix: josdejong#3073 function `format` not escaping control characters and double quotes in strings * chore: add more unit tests * feat: implement subtractScalar (josdejong#3081, josdejong#2643) * added subtractScaler * added subtractScaler missing entries * added test cases for 2 or more parameters, test for subtractScalar instead fo subtract * replaced subtract with subtractScalar whereever possible --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * fix: function `clone` not throwing an error in case of an unsupported type like a function * chore: make the unit test more robust * fix: josdejong#2960 add type definition of function `symbolicEqual` (josdejong#3035) * chore: update devDependencies * chore: publish v11.12.0 * fix: josdejong#2919 TypeScript types not working with NodeNext module resolution (josdejong#3079) * docs: update deprecation messages * chore: publish v12.0.0 * chore: update history (forgot to mention a feature in v12) * fix: josdejong#3088 error in the description of the return type of `pickRandom` * fix josdejong#3087: extend function `mod` with support for negative divisors in when using `BigNumber` or `Fraction` * fix josdejong#3092: a typo in an error message when converting a string into a number * fix josdejong#3094: function `derivative` mutates the input expression when it fails * feat: extend function `round` with support for units (josdejong#3095) * fix josdejong#2761: implement support for units in function `round` (WIP) * fix josdejong#2761: extend function `round` with support for units * docs: describe all signatures in the docs of function round * chore: fix linting issue * chore: remove less-useful signatures for round with units and matrices * chore: update devDependencies * chore: publish v12.1.0 * docs: update the release date in history.md * fix: josdejong#3096 embedded docs of `eigs` throwing an error * chore: update history * fix: accidentally passing a scope as third _and_ fourth argument to raw functions * feat: lazy evaluation of and, or, &, | (josdejong#3101, josdejong#3090) * If fn has rawArgs set, pass unevaluated args * Add shared helper function for evaluating truthiness * Add and & or transform functions for lazy evaluation * Add lazy evaluation of bitwise & and | operators * Add unit tests for lazy evaluation * Add lazy evaluation note to docs * Move documentation to Syntax page * Replace `testCondition()` with test evaluation of logical function itself * Use `isCollection()` to simplify bitwise transform functions * fix: do not copy scope in raw OperatorNode, test lazy operators scope * fix: linting issues --------- Co-authored-by: Brooks Smith <brooks.smith@clearcalcs.com> * docs: update history * chore: update devDependencies * chore: publish `v12.2.0` * fix: josdejong#3109 method `Node.toHTML` not accepting a custom `handler` * chore: upgrade node and Github actions versions for CI * chore: upgrade devDependencies * chore: publish v12.2.1 * chore: oopsie, update version number in version.js * chore: up version number, and pin fraction.js at v4.3.4 * chore: publish v12.2.1 * docs: update maintenance badge to 2024 * docs: fix the github sponsors badge * Support new metric prefixes: Q, R, r, and q (josdejong#3113) * added Q, R, r, q metrix prefixes * tests added for new prefixes * removed duplicate tests * maybe square and cubic tests will bump code cov into the positive * Check numeric value --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * docs: change 2023 to 2024 * Unitless quantity conversion bug (josdejong#3117) * Add test for conversion to unitless quantity * Avoid access to missing array index * Also check that other.units is not empty * Add test for abs of dimensionless unit * Fix: avoid access to missing units array index --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * fix `toSI()` wrongly converting `degC` (josdejong#3118) * Add new test for degC toSI * Convert value using to() if needed * Only set ret.value = null when it is not already null --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * chore: update devDependencies * chore: publish v12.3.0 * chore: run `npm audit fix` * Infer types of arguments more precisely (josdejong#3123) * Prefer inferring types of nodes as tuples * Implement for IndexNode * Test for types * Use tuple type for array node --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * CodeEditorExample (josdejong#3027) * broadcasting * Simplified broadcasting * Updated for broadcasting * Changed to camel case * Camel case and auto formating * Added comments * Skip if matrices have the same size * Fixed issue with undefined variable missing dot in `A._size` * Implemented broadcasting in all functions * Added helper functions * Added function to check for broadcasting rules * Tests for broadcasted arithmetic * Fixed issue with matrix the size of a vector * Documented and updated broadcasting * Included broadcast.test * Included math to syntax when missing * Add code editor example * Vite mini project * Initial example * added alpine debounce * Fixed display * Added parser.clear * Added mathjs-language * Made module to get expressions * Added custom events * Issue with help formatting * Simplified help format * Restored package.json * removed unneded icons * Added readme file * Fixed versions * Commented getExpressions * Documented main.js * Fixed title * Fixed alpine version * Removed AlpineJS * Added documentation and renamed variables for clarity * Fixed naming errors --------- Co-authored-by: David Contreras <david.contreras@guentner.com> Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: minor refinement in the `code editor` example * chore: update HISTORY.md * fix: josdejong#3114 build warnings related to a number of wrong `/* #__PURE__ */` annotations * chore: do not output documentation warnings unless running with a `--debug-docs` flag * docs: update authors * Fixed issue with long lines in Code Editor Example (josdejong#3130) * broadcasting * Simplified broadcasting * Updated for broadcasting * Changed to camel case * Camel case and auto formating * Added comments * Skip if matrices have the same size * Fixed issue with undefined variable missing dot in `A._size` * Implemented broadcasting in all functions * Added helper functions * Added function to check for broadcasting rules * Tests for broadcasted arithmetic * Fixed issue with matrix the size of a vector * Documented and updated broadcasting * Included broadcast.test * Included math to syntax when missing * Add code editor example * Vite mini project * Initial example * added alpine debounce * Fixed display * Added parser.clear * Added mathjs-language * Made module to get expressions * Added custom events * Issue with help formatting * Simplified help format * Restored package.json * removed unneded icons * Added readme file * Fixed versions * Commented getExpressions * Documented main.js * Fixed title * Fixed alpine version * Removed AlpineJS * Added documentation and renamed variables for clarity * Fixed naming errors * Fixed issue with long lines * Edge case where multiple expressions are on the same line not ending in ";" --------- Co-authored-by: David Contreras <david.contreras@guentner.com> Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * docs: josdejong#3145 fix documentation about REPL, it does require a build step nowadays * fix: josdejong#3142 support BigNumber values for the options of function `format`: `precision`, `wordSize`, `lowerExp`, `upperExp` * Improve type definitions of function `hypot` (josdejong#3144) * Addressed silentmissile's comment in josdejong#3125 * Added method overload to index.d.ts, have to revert commit due to changes to package-lock.json with using npm install to run the unit tests & lint tests * chore: update HISTORY.md * fix: josdejong#3141 `help(config)` altering the actual `config` when evaluating the examples * chore: update devDependencies * chore: publish `v12.3.1` * chore: add a benchmark to get a feel for how fast scope variables are resolved * chore: fix linting issue * Fix not being able to use `and` and `or` inside a function definition (josdejong#3150) * chore: write unit tests using `and` and `or` inside a function definition (WIP) * fix: josdejong#3143 fix scope issues in rawArgs functions by implementing a `PartitionedMap` * fix: add more unit tests for `ObjectWrappingMap` * fix: don't let `ObjectWrappingMap` and `PartitionedMap` extend `Map` (risk of having non-overwritten methods) * docs: update docs about `rawArgs` functions * chore: update devDependencies * chore: publish v12.3.2 * chore: publish v12.3.2 * fix: josdejong#3155 remove an outdated section about complex numbers from the docs * docs: describe `getAllAsMap` in the Parser docs (josdejong#3158) * chore: update history * Determinant with small numbers fix (josdejong#3139) * feat: trailing commas in matrices (josdejong#3154) * chore: update history * docs: fix broken example in the documentation about matrices (see josdejong#3159) * fix: `PartitionedMap` and `ObjectWrappingMap` missing a property `Symbol.iterator` * fix: linting issue * fix: mode signature return types (josdejong#3153) * fix: mode type signatures * Add ts tests for mode * Add assertions mode type tests * Update author Rich in AUTHORS file --------- Co-authored-by: Rich Martinez <richmartinez@Edinas-MacBook-Pro.local> Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * feat: improve the performance f `multiply` by adding matrix type inferencing (josdejong#3149) * added type inference * added back accidentally removed return statement and made it so that the explicitly defined type is returned at the end * made sure that mixed types are ignored in the process data types check * fixed issue with undefined _data for SparseMatrix and linting issues * simplified syntax and added type inferencing to src/type/matrix/utils and src/function/matrix/dot.js * shortened the final part of the type inferencing and moved it to matrix creation in multiply --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * Fix: josdejong#3100 function `round` not handling round-off errors (josdejong#3136) * Fixing rounding bug from issue 3100 * Corrected syntax and converted if...else to logic using ternary operator * Removing nearlyEqual comparison because a false return value was mathematically impossible by user input. Adding dynamic epsilon logic to cover cases when a user requests to round a number to a higher precision than epsilon in the config file. Also adding tests to cover dynamic epsilon cases. * Removing dynamic epsilon and adding test for changing config.epsilon during runtime * Reintroducing nearly equal verification for round function. Adding test case for changing epsilon at runtime. Both tests for changing epsilon at runtime also verify the false nearlyEqual scenario. --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * chore: update devDependencies (most notably eslint) * chore: publish v12.4.0 * fix josdejong#3163: `toTex` wrongly returning `Infinity` for large BigNumbers * fix josdejong#3162: add license information about CSParse (josdejong#3164) * update history * fix: faster startup time of the CLI and REPL by loading the bundled file * feat: Interactive lorenz example (josdejong#3151) * Interactive lorenz * Separate Interactive Lorenz * Cleanup * Bigger graphs * Full screen examples --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * fix: give the inputsDiv a white background (see josdejong#3151) * chore: update history * chore: remove `polyfill.io` inside example (josdejong#3167) Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * fix josdejong#3175: expose `math.Unit.ALIASES`, update history * chore: update history * doc: create CODE_OF_CONDUCT.md See josdejong#3174 * fix: josdejong#3172 simplify `"true and true"` * fix: josdejong#3175 cannot delete units using `math.Unit.deleteUnit` * chore: update devDependencies * chore: run `npm audit fix` * chore: publish v12.4.1 * docs: fix misleading documentation for expression tree traverse (josdejong#3177) Callback function for MathNode.traverse() returns void. Documentation says callback must return a replacement for the existing node (possibly copied from transform() above). * chore: update history * fix: josdejong#3180 fix type definitions of function `add` and `multiply` to allow more than two arguments * chore: update devDependencies (most notably `gulp@5`) * chore: replace utility function `values` with `Object.values` (fix josdejong#3194) * fix josdejong#3192: function `isNaN` returns `false` for `NaN` units in a matrix or array * Use referToSelf() to recursively check if various types are NaN in an array or matrix * fix array test description from isNegative to isNaN * Add test for units in a matrix --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * chore: update devDependencies * chore: publish `v12.4.2` * chore: replace util functions `values` and `contains` with using native JS functions (see josdejong#3194) * chore: replace util functions `values` and `contains` and usages of `indexOf` with using native JS functions `values` and `contains` (see josdejong#3194) * fix: serialization of Units without a value, see josdejong#1240 * Fix: outdated, incorrect documentation about the order of precedence for operator modulus `%`. See josdejong#3189 * feat: nearly equal with relative and absolute tolerance (josdejong#3152) * nearlyEqual with absolute and relative tolerances * Format * nearlyEqual for bigNumber * Added skip for NaN * Reduce diff a bit * Issue with examples in jsdcos * Updated all calls for nearlyEqual * Fixed failing tests * Changed epsilon to relTol, absTol * Changed references to epsilon in docs and tests * Added warning for config.epsilon * Fix warning in zeta.test * Added config test * Added sinon to test console.warn --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: move `sinon` to devDependencies and fix two typos * chore: adjust `isPositive`, `isNegative`, and `isZero` to the new `relTol` and `absTol` * docs: document how to run tests for the type definitions * Improve quantileSeq typings (josdejong#3198) * Improve quantileSeq typings * Add tests, revert comment changes * Fix type tests * chore: update HISTORY.md * chore: cleanup entry files that are deprecated since `v8.0.0` (2020-11-06) * fix: upgrade to `fraction.js@4.3.7` * chore: convert CJS files to ESM (josdejong#3204) * chore: added test cases to deepForEach (josdejong#3211) * feat: implement support for `bigint` (josdejong#3207, josdejong#2737) * chore: move browserslist from package.json into `.browserslistrc` * chore: change browerslist to browsers that are not dead and fully support es6 * chore: improve browserslist to explicity require bigint support * chore: publish v12.4.3 * chore: update package-lock.json * chore: update devDependencies * chore: publish v13.0.0 * docs: document dropping JS engines that do not support E6 or bigint in v13 * fix: example advanced/custom_argument_parsing.js * chore: add unit tests for `deepMap`, `isCollection`, and `reduce` * docs: fix example `convert_fraction_to_bignumber.js` by upgrading to `typed-function@4.2.1` * Broadcast refactor (josdejong#3220) * chore: update history * fix: josdejong#3227 generated bundle containing `catch` blocks without parameters * fix: josdejong#2348 update type definitions of the `Parser` methods (josdejong#3226) Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update devDependencies and run `npm audit fix` * chore: publish v13.0.1 * Further improve quantileSeq typings (josdejong#3223) Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * chore: update devDependencies * fix josdejong#3227: change the minimum required JS version to ES2020 in the docs * chore: publish v13.0.2 * chore: update dependencies of the code editor example * fix: josdejong#3232 fix type definitions of function `format` to support notations `hex`, `bin`, and `oct` * fix: use exact values for US liquid volume units (josdejong#3229) 1 US gallon is defined as 231 cubic inches, which is exactly 3.785411784 L (since 1 inch is defined as 25.4 mm). Other units are defined against the gallon. Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * fix: types static methods and members for Unit class (josdejong#3230) * fix: types static method for Unit class Changes unit interface to declare class to enable the adding of static methods. * refactor: change to not using declare class * fix: adds more unit static methods and updates tests * chore: moves test from wrong location * fix: adds additional static methods and updates jsDocs --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * chore: update devDependencies * chore: publish `v13.0.3` * chore: update package-lock.json * chore: revert updating devDependencies * chore: revert reverting updating devDependencies * chore: try use `ubuntu-24.04` instead of `ubuntu-latest` * chore: try use `ubuntu-22.04` instead of `ubuntu-24.04` * chore: try use `ubuntu-latest` instead of `ubuntu-22.04` again * chore: disable testing on Node 22 for now until we get mocha working again in GitHub actions * chore: publish `v13.0.3` for real * chore: enable testing on Node 22 again * feat: add matrix datatypes in more cases (josdejong#3235) * chore: update history * docs: add a link to the documentation page about the syntax expression from the function `evaluate` (fix josdejong#3238) * feat: export util functions for maps and improve documentation of `scope` (josdejong#3243) * feat: export util functions `isMap`, `isPartitionedMap`, and `isObjectWrappingMap` and improve the documentation of `scope` (see josdejong#3150) * chore: fix broken unit tests * docs: refine the explanation about scopes * chore: update history * fix: josdejong#3244 fix broken link to `ResultSet` in the docs about classes * fix: function `map` not always working with matrices (josdejong#3242) * Removed maxArgumentCount in favor of applyCallback * Making a pure _recurse function * Added cbrt tests, removed unnecesary changes in functions. * Fixed main bottleneck * Restored back function before unintended change * Fix format --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * chore: add tea.yaml file * docs: spelling fixes in the embedded docs (josdejong#3252) Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * chore: add a benchmark testing `DenseMatrix.map(...)` and `DenseMatrix.forEach(...)` (see josdejong#3251) * feat: support multiple inputs in function `map` (josdejong#3228) * chore: update history * chore: update devDependencies * chore: publish `v13.1.0` * fix: various security vulnerabilities (josdejong#3255) * fix: disable parser functions in the CLI (security issue) * fix: ensure `ObjectWrappingMap` doesn't allow deleting unsafe properties (security issue) * fix: enable using methods and (safe) properties on plain arrays * docs: update the "Less vulnerable expression parser" section in the docs * chore: fix typos and linting issues * chore: keep functions like `simplify` enabled in the CLI * docs: update the security page * fix: ensure `ObjectWrappingMap.keys` cannot list unsafe properties * fix: when overwriting a rawArgs function with a non-rawArgs function it was still called with raw arguments * docs: fix a typo * chore: publish v13.1.1 * fix broken links in configuration.md (josdejong#3254) Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * fix: improve the type definitions of `ConstantNode` to support all data types (josdejong#3257) * chore: update history * chore: fix broken benchmark * fix: josdejong#3259 function `symbolicEqual` missing in the TypeScript definitions * chore: update AUTHORS file * fix: josdejong#3259 revert the duplicate `symbolicEqual` definition and just export the existing definitions * fix: josdejong#3246 add type definitions for function `leafCount` * fix: josdejong#3253 cannot use identifiers containing special characters in function `derivative` * chore: update history * chore: extend the `map.js` benchmark * chore: fix linting issue * chore: improve performance of functions `map`, `filter` and `forEach` (josdejong#3256) * Implement reduceCallback * Add jsdocs * implement simplifyCallback in other functions * Moved recurse to array.js * Format * Separate transform callback * forEach transform * Renamed applyCallback to simplifyCallback * Simplified index transform * renamed to reducedCallback and simplifiedCallback to simpleCallback * chore: fix linting issue * Added forEach benchmark * renamed simplifyCallback to optimizeCallback --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update history * fix: josdejong#3267 implicit multiplication with a negative number and unit `in` * feat: speed up the `map()` and `forEach()` functions in DenseMatrix.js (josdejong#3251) * Optimize the map and forEach functions in DenseMatrix.js * Changed index back to Array from Uint32Array and clone it using index.slice(0) instead of [...index] * Fixed merge conflicts with the fast callback optimization * Fixed the documentation for _forEach() * Fixed _forEach comment and made it return an immutable index array * Resolved DenseMatrix unit test suggestions --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * chore: update HISTORY.md and AUTHORS * chore: use `codecov/codecov-action` * chore: try fix the codecov-action * chore: try fix the codecov-action * chore: try fix the codecov-action * chore: try fix the codecov-action * docs: document the syntax of `map` and `forEach` in the expression parser (josdejong#3272) * chore: update docs * chore: update devDependencies * chore: publish `v13.2.0` * chore: add a missing comma * docs: fix a typo on the Syntax page (josdejong#3276) * fix: update dependencies and devDependencies * chore: cleanup unused imports * chore: revert to `typescript@5.5.4` to keep the (old) eslint version happy --------- Co-authored-by: Glen Whitney <glen@studioinfinity.org> Co-authored-by: Jos de Jong <wjosdejong@gmail.com> Co-authored-by: Vincent Tam <VincentTam@users.noreply.github.com> Co-authored-by: Vrushaket Chaudhari <82214275+vrushaket@users.noreply.github.com> Co-authored-by: Juan Pablo Alvarado <63080419+juancodeaudio@users.noreply.github.com> Co-authored-by: Brooks Smith <brooks.smith@clearcalcs.com> Co-authored-by: Alex Edgcomb <aedgcomb@gmail.com> Co-authored-by: Carl Osterwisch <costerwi@gmail.com> Co-authored-by: S.Y. Lee <sylee957@gmail.com> Co-authored-by: David Contreras <dvd.cnt@gmail.com> Co-authored-by: David Contreras <david.contreras@guentner.com> Co-authored-by: Hudsxn <143907857+Hudsxn@users.noreply.github.com> Co-authored-by: Rich Martinez <6185506+rich-martinez@users.noreply.github.com> Co-authored-by: Rich Martinez <richmartinez@Edinas-MacBook-Pro.local> Co-authored-by: RandomGamingDev <83996185+RandomGamingDev@users.noreply.github.com> Co-authored-by: Brian Fugate <fugateb@yahoo.com> Co-authored-by: Sukka <isukkaw@gmail.com> Co-authored-by: Rohil Shah <shah5963@gmail.com> Co-authored-by: Laurent Gérin <41303636+lgerin@users.noreply.github.com> Co-authored-by: Adam Jones <domdomegg+git@gmail.com> Co-authored-by: Lucas Eng <lucaseng19@gmail.com> Co-authored-by: Orel Ben Neriah <77707952+orelbn@users.noreply.github.com> Co-authored-by: Vistinum <eugene225@gmail.com> Co-authored-by: Vas Sudanagunta <vas@commonkarma.org> Co-authored-by: Brooks Smith <42363318+smith120bh@users.noreply.github.com> Co-authored-by: Jmar L. Pineda <63294460+Galm007@users.noreply.github.com>

RandomGamingDev added 4 commits February 6, 2024 19:44

added type inference

458314a

added back accidentally removed return statement and made it so that …

b5581c5

…the explicitly defined type is returned at the end

made sure that mixed types are ignored in the process data types check

ed438be

fixed issue with undefined _data for SparseMatrix and linting issues

ed58386

RandomGamingDev mentioned this pull request Feb 12, 2024

Using a separate math library for handling p5.js's math operations processing/p5.js#6527

Open

17 tasks

josdejong reviewed Feb 16, 2024

View reviewed changes

src/function/arithmetic/multiply.js Outdated Show resolved Hide resolved

src/function/arithmetic/multiply.js Show resolved Hide resolved

src/function/arithmetic/multiply.js Outdated Show resolved Hide resolved

simplified syntax and added type inferencing to src/type/matrix/utils…

8369586

… and src/function/matrix/dot.js

Merge branch 'develop' into develop

842c5db

josdejong reviewed Feb 21, 2024

View reviewed changes

src/function/arithmetic/multiply.js Outdated Show resolved Hide resolved

src/function/arithmetic/multiply.js Outdated Show resolved Hide resolved

src/function/arithmetic/multiply.js Outdated Show resolved Hide resolved

RandomGamingDev and others added 2 commits February 21, 2024 14:22

Merge branch 'josdejong:develop' into develop

d4e434f

shortened the final part of the type inferencing and moved it to matr…

44d9dd4

…ix creation in multiply

josdejong approved these changes Feb 22, 2024

View reviewed changes

josdejong merged commit 207623c into josdejong:develop Feb 22, 2024
9 checks passed

RandomGamingDev mentioned this pull request Aug 14, 2024

Speed up the map() and forEach() functions in DenseMatrix.js by over 300% #3251

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added type inferencing for Vector & Matrix operations in multiply.js (+performance boost!) #3149

Added type inferencing for Vector & Matrix operations in multiply.js (+performance boost!) #3149

RandomGamingDev commented Feb 7, 2024

josdejong commented Feb 7, 2024

RandomGamingDev commented Feb 7, 2024

gwhitney commented Feb 7, 2024 •

edited

Loading

RandomGamingDev commented Feb 7, 2024 •

edited

Loading

gwhitney commented Feb 7, 2024

RandomGamingDev commented Feb 7, 2024 •

edited

Loading

gwhitney commented Feb 7, 2024

RandomGamingDev commented Feb 7, 2024 •

edited

Loading

josdejong commented Feb 9, 2024

RandomGamingDev commented Feb 9, 2024 •

edited

Loading

josdejong left a comment

RandomGamingDev commented Feb 18, 2024 •

edited

Loading

josdejong left a comment

josdejong left a comment

josdejong commented Feb 22, 2024

Added type inferencing for Vector & Matrix operations in multiply.js (+performance boost!) #3149

Added type inferencing for Vector & Matrix operations in multiply.js (+performance boost!) #3149

Conversation

RandomGamingDev commented Feb 7, 2024

The Issue

mathjs has had an "issue" with performance (some of the first search results for "mathjs is slow":

mathjs is quite powerful, but the idea that you should explicitly set the numeric type of your tensors is something that people simply don't come across when reading the documentation (here are some pages from the documentation):

In fact, mathjs's documentation makes it look like explicitly defining the numeric type is completely unnecessary because of the default type assignment in the config of mathjs:

This leads to extremely poor performance when people try to use mathjs:

Which wouldn't exist had these explicit numeric types been used:

Why this wasn't done before now and how this PR fixes the issue that caused it

It seems like adding type inferencing was previously considered, but then ignored because of an issue with changing element types:

This PR fixes this by getting the type during the tensor operation itself

The Solution this PR provides

Type inferencing boosts the performance to speeds nearly indistinguishable from the speeds of explicit types

josdejong commented Feb 7, 2024

RandomGamingDev commented Feb 7, 2024

gwhitney commented Feb 7, 2024 • edited Loading

RandomGamingDev commented Feb 7, 2024 • edited Loading

gwhitney commented Feb 7, 2024

RandomGamingDev commented Feb 7, 2024 • edited Loading

gwhitney commented Feb 7, 2024

RandomGamingDev commented Feb 7, 2024 • edited Loading

josdejong commented Feb 9, 2024

RandomGamingDev commented Feb 9, 2024 • edited Loading

josdejong left a comment

Choose a reason for hiding this comment

RandomGamingDev commented Feb 18, 2024 • edited Loading

josdejong left a comment

Choose a reason for hiding this comment

josdejong left a comment

Choose a reason for hiding this comment

josdejong commented Feb 22, 2024

gwhitney commented Feb 7, 2024 •

edited

Loading

RandomGamingDev commented Feb 7, 2024 •

edited

Loading

RandomGamingDev commented Feb 7, 2024 •

edited

Loading

RandomGamingDev commented Feb 7, 2024 •

edited

Loading

RandomGamingDev commented Feb 9, 2024 •

edited

Loading

RandomGamingDev commented Feb 18, 2024 •

edited

Loading